feat(node): probe foreign-chain RPC providers on startup - #3848
feat(node): probe foreign-chain RPC providers on startup#3848haiyuechen-nearone wants to merge 1 commit into
Conversation
131a75f to
e9e4b81
Compare
05a55bc to
ad30f88
Compare
4eeed22 to
8cdfbfa
Compare
ad30f88 to
a1145b2
Compare
a1145b2 to
9031612
Compare
8cdfbfa to
36e439e
Compare
a2805eb to
3817516
Compare
30c77ea to
208c837
Compare
208c837 to
7958f4b
Compare
Pull request overviewAdds a startup health check that probes every configured foreign-chain RPC provider once against a golden reference transaction, then logs per-provider results and a Changes:
Reviewed changesPer-file summary
FindingsNon-blocking (nits, follow-ups, suggestions):
✅ Approved |
8e97b7e to
47f58d5
Compare
| pub fn resolve_network_from_config( | ||
| near_init: Option<&NearInitConfig>, | ||
| contract_id: &str, | ||
| ) -> NetworkKind { | ||
| match near_init { | ||
| Some(near_init) => match near_init.chain_id { | ||
| ChainId::Mainnet => NetworkKind::Public(Network::Mainnet), | ||
| ChainId::Testnet => NetworkKind::Public(Network::Testnet), | ||
| ChainId::Localnet | ChainId::Sandbox | ChainId::Custom(_) => NetworkKind::Local, | ||
| }, | ||
| None => match network_from_contract_id(contract_id) { | ||
| Some(network) => NetworkKind::Public(network), | ||
| None => NetworkKind::Undetermined, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| /// Classify mainnet/testnet from a contract-id suffix. | ||
| pub fn network_from_contract_id(contract_id: &str) -> Option<Network> { | ||
| if contract_id.ends_with(".testnet") { | ||
| Some(Network::Testnet) | ||
| } else if contract_id.ends_with(".near") || contract_id == "v1.signer" { | ||
| Some(Network::Mainnet) | ||
| } else { | ||
| None | ||
| } | ||
| } |
There was a problem hiding this comment.
To reviewers: please double check that I have the correct understanding of our network and contract naming conventions
netrome
left a comment
There was a problem hiding this comment.
Started looking at this now, some initial thoughts:
- We should not expect RPCs to retain individual TX information forever (blocking)
- I'd prefer to minimize network-specific code paths. Ideally we should not have any network-specific code paths, but I can see the point of providing defaults for mainnet and testnet for convenience.
| // A config-supplied golden is only meaningful on a local chain; on | ||
| // mainnet/testnet the built-in set is always used. |
There was a problem hiding this comment.
Why don't we provide golden configurations on mainnet/testnet? Hard-coding values feels like a time bomb, as many RPCs might not serve requests for old transactions.
There was a problem hiding this comment.
My understanding was that we want to always check for the same transaction, which is baked in to the binary, maybe I misunderstood.
@anodar Judging from the foreign chain tester CLI code I assumed that we would like to continue using hard coded golden transactions for health check, maybe you had something else in mind?
| "foreign_chain_health_check_golden is set in config but ignored on \ | ||
| mainnet/testnet; the built-in golden set is always used — remove it \ | ||
| from config.yaml" |
There was a problem hiding this comment.
Wouldn't it make sense to override the golden set if provided manually? The hard-coded values could exist as a convenience so we don't have to ask all operators to manually add these but retaining the ability to always override sounds like it would simplify our code. I'd prefer if we have as few network-specific code paths as possible. Ideally none.
| /// Hashes are hex, with or without a `0x` prefix. | ||
| #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] | ||
| pub struct BlockHashGolden { | ||
| pub tx: String, | ||
| pub block_hash: String, | ||
| } |
There was a problem hiding this comment.
I think this is a dangerous construction. Many RPCs won't serve transactions forever. It's better to ask for genesis hash or chain ID, like we do for Sui.
Spawn a detached, diagnostic-only startup task that concurrently probes every configured foreign-chain RPC provider against its expected chain identity and logs a per-provider result plus an `x/y providers healthy` summary — so config typos and un-enabled API keys surface at startup instead of on the first real verification request. The probe never blocks startup, and failure reasons are not logged (they can carry secrets). Expected identities are read from the node's typed config (`foreign_chain_health_check.identities`); `ExpectedIdentities` moves to node-config so the node and the config-tester share one type.
485eb48 to
315514c
Compare
Part of #3764. Wires the identity-based check into the node. Merge the identity-probe stack (#3932 → #3962 → #3963 → #3964) first, then this trio bottom-up:
configured/healthyPrometheus gauges/debug/node_configSpawns a detached startup task that concurrently probes every configured foreign-chain RPC provider once — verifying its chain identity and inspecting a recent transaction, the same inspector and auth the real verification path uses — and logs a per-provider result plus an
x/y providers healthysummary. The probe never blocks startup, and failure reasons aren't logged (they can carry secrets).Expected identities come from
foreign_chain_health_check.identities(no built-ins, so any network — including local — is checkable; a configured chain with no identity fails).ExpectedIdentitiesmoves to node-config so the node and config-tester share one type.